-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Regression: Allow toHaveBeenCalledWith and related functions to test calls with no arguments #1951
Conversation
arguments This behavior broke in Jest 16, so adding a test to document it for the future. Also renamed another test that appeared to cover this case but did not.
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
I would love some pointers in terms of fixing this issue. I dug into it a bit last week, but the code around the spy matchers seems to have been heavily refactored in the past release, and I couldn't trace down the regression. I'd be happy to take a pointer to where the code determines which arguments are passed to the matcher, and try to flesh this PR out from there |
@cpojer yeah, I got to that point earlier. When that function is called in my basic test
calls is set to What I'm unclear on is where the matcher is called from and what would cause |
That should be somewhere around here: https://github.com/facebook/jest/blob/master/packages/jest-matchers/src/index.js#L55 we rewrote this matcher completely as we are trying to kill parts of Jasmine that we don't like :) |
Code was previously assuming at least one argument, but that assumption failed when a function was called with no args.
@cpojer Ah perfect, code was assuming at least one expected argument (function signature of |
that makes sense! Thank you. |
…est calls with no arguments (jestjs#1951) * Add failing test for expecting a function to have been called with no arguments This behavior broke in Jest 16, so adding a test to document it for the future. Also renamed another test that appeared to cover this case but did not. * Allow testing whether a test has been called with 0 arguments Code was previously assuming at least one argument, but that assumption failed when a function was called with no args.
…est calls with no arguments (jestjs#1951) * Add failing test for expecting a function to have been called with no arguments This behavior broke in Jest 16, so adding a test to document it for the future. Also renamed another test that appeared to cover this case but did not. * Allow testing whether a test has been called with 0 arguments Code was previously assuming at least one argument, but that assumption failed when a function was called with no args.
…est calls with no arguments (jestjs#1951) * Add failing test for expecting a function to have been called with no arguments This behavior broke in Jest 16, so adding a test to document it for the future. Also renamed another test that appeared to cover this case but did not. * Allow testing whether a test has been called with 0 arguments Code was previously assuming at least one argument, but that assumption failed when a function was called with no args.
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
This PR adds a failing test for the issue identified in #1899 to help expose the issue and prevent future regressions.
Test plan
running
npm test
in the repo with this change causes tests to fail for now, as expected since Jest16 regresses when testing functions that have been called without arguments.